Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More explanation for OTLP transport protocols #1790

Merged
merged 5 commits into from
Jul 12, 2021

Conversation

iNikem
Copy link
Contributor

@iNikem iNikem commented Jul 5, 2021

Fixes #1764

Changes

More explanation for OTLP transport protocols. Also clearly state that the support for one of the grpc or http/protobuf is required and http/json is not enough.

@iNikem iNikem requested review from a team July 5, 2021 05:12
@iNikem iNikem force-pushed the exporter-transport branch from 0a844e8 to f428e8c Compare July 5, 2021 05:13
- `http/protobuf` for protobuf-encoded data over HTTP connection
- `http/json` for JSON-encoded data over HTTP connection

SDKs MUST support either `grpc` or `http/protobuf` and SHOULD support both. They also MAY support `http/json`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't seen a definitive analysis in the ticket of why one of protobuf-based protocols is a MUST, can you elaborate?

Specifically, in the past we've always had issues with supporting Thrift in the browser - I don't know the current state of protobuf for browser, but according to #1764 (comment) it's a similar story, JSON is preferred.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#1764 (comment) and #1764 (comment)

It was also discussed during the last maintainers meeting (https://youtu.be/ZqQ4sbOsvLs?t=887) and this decision was made there.

Copy link
Contributor

@anuraaga anuraaga Jul 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps instead of SDKs, Languages is more obvious? I suppose the intent is that JS supports at least protobuf for node users, and if protobuf is prohibitive in a browser it can also support JSON for that specific use case.

@carlosalberto
Copy link
Contributor

I think we are ready to go. @iNikem would you mind adding a Changelog entry? Thanks!

@iNikem
Copy link
Contributor Author

iNikem commented Jul 12, 2021

I think we are ready to go. @iNikem would you mind adding a Changelog entry? Thanks!

@carlosalberto done.

@carlosalberto carlosalberto merged commit 862a1a3 into open-telemetry:main Jul 12, 2021
carlosalberto pushed a commit to carlosalberto/opentelemetry-specification that referenced this pull request Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Confusing transport protocols
6 participants